Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[configuration] delayed postprocs, extra volmounts #738

Merged
merged 1 commit into from
Dec 18, 2021

Conversation

relrod
Copy link
Member

@relrod relrod commented Dec 14, 2021

I've pulled this out from my lint work, where it was needed, because it feels like it should be a separate change because we probably want it regardless.

This allows post processors to be delayed until after the first pass of post processors has been completed.
At that point, it will run the delayed ones. There is only one "level" of delay (so delayed post processors can't wait for a third pass). This is intentional, to keep things simple, otherwise we'd have a whole tree of post processors and it gets complex quickly.

Doing it this way just provides a simple way to say "this post processor depends on some other things that happen in the first pass, let's wait for this before we actually run it."

Change:

- Allow post-processors to be delayed until after the first
  (alphabetical) pass. In the delayed pass, delayed post-processors
  are again executed alphabetically.

- In the interest of keeping configuration simple, we don't allow for
  multiple "levels" of delay. A post-processor is either run during the
  first pass, or delayed until after the first/normal pass.

- This also adds an "extra volmounts" variable to the navigator
  post-processor class, which can be used by other post-processors to
  signify that they want a volume to be mounted. The volume-mount post
  processor is now delayed to accommodate.

Test Plan:

- Using this in lint/navigator work, no new tests yet because there is
  no other post processor currently which adds an extra volume mount.

Signed-off-by: Rick Elrod <rick@elrod.me>

Copy link
Collaborator

@cidrblock cidrblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool. I was wondering how we could bypass the alphabetical order thing, a send pass is a neat idea. Couple suggestions, nothing major.

The only other thing I was thinking about it a future developer might be able to add a new volume mount in a incorrect format. We could create a SimpleNamespace as a carrier that had it's own function to turn itself into a string.

If we set the type of extra_volume_mounts to List[VolumeMount] mypy would catch it in the future.

Later on, we could eliminate some of the logic in the ee volume mount post processor by passing the dicts through the new simplenamespace SimpleNamespace(**d).to_string()

just thinking aloud here about a mistake I could make in the future :)

@ssbarnea
Copy link
Member

@relrod Please ensure zuul check is passing, otherwise we will not be able merge it.

@relrod
Copy link
Member Author

relrod commented Dec 15, 2021

recheck

1 similar comment
@relrod
Copy link
Member Author

relrod commented Dec 15, 2021

recheck

@relrod
Copy link
Member Author

relrod commented Dec 15, 2021

this is fun

@relrod
Copy link
Member Author

relrod commented Dec 18, 2021

recheck

Change:

- Allow post-processors to be delayed until after the first
  (alphabetical) pass. In the delayed pass, delayed post-processors
  are again executed alphabetically.

- In the interest of keeping configuration simple, we don't allow for
  multiple "levels" of delay. A post-processor is either run during the
  first pass, or delayed until after the first/normal pass.

- This also adds an "extra volmounts" variable to the navigator
  post-processor class, which can be used by other post-processors to
  signify that they want a volume to be mounted. The volume-mount post
  processor is now delayed to accommodate.

Test Plan:

- Using this in lint/navigator work, no new tests yet because there is
  no other post processor currently which adds an extra volume mount.

Signed-off-by: Rick Elrod <rick@elrod.me>
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ansible-zuul ansible-zuul bot merged commit a9bd34c into ansible:main Dec 18, 2021
cidrblock pushed a commit to cidrblock/ansible-navigator that referenced this pull request Jan 11, 2022
[configuration] delayed postprocs, extra volmounts

I've pulled this out from my lint work, where it was needed, because it feels like it should be a separate change because we probably want it regardless.
This allows post processors to be delayed until after the first pass of post processors has been completed.
At that point, it will run the delayed ones. There is only one "level" of delay (so delayed post processors can't wait for a third pass). This is intentional, to keep things simple, otherwise we'd have a whole tree of post processors and it gets complex quickly.
Doing it this way just provides a simple way to say "this post processor depends on some other things that happen in the first pass, let's wait for this before we actually run it."
Change:

- Allow post-processors to be delayed until after the first
  (alphabetical) pass. In the delayed pass, delayed post-processors
  are again executed alphabetically.

- In the interest of keeping configuration simple, we don't allow for
  multiple "levels" of delay. A post-processor is either run during the
  first pass, or delayed until after the first/normal pass.

- This also adds an "extra volmounts" variable to the navigator
  post-processor class, which can be used by other post-processors to
  signify that they want a volume to be mounted. The volume-mount post
  processor is now delayed to accommodate.

Test Plan:

- Using this in lint/navigator work, no new tests yet because there is
  no other post processor currently which adds an extra volume mount.

Signed-off-by: Rick Elrod <rick@elrod.me>

Reviewed-by: Bradley A. Thornton <bthornto@redhat.com>
Reviewed-by: Sorin Sbarnea <sorin.sbarnea@gmail.com>
Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/profile@redhat.com>
Reviewed-by: Rick Elrod <rick@elrod.me>
Reviewed-by: None <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants